Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

PVF: instantiate runtime from bytes #7270

Merged
merged 3 commits into from
May 23, 2023

Conversation

mrcnski
Copy link
Contributor

@mrcnski mrcnski commented May 22, 2023

PULL REQUEST

Overview

Summary of rationale:

  1. Removes filesystem access from the PVF thread so we can lock it down.
  2. Removes a potential race where we were checking file existence before passing the file path to create_runtime_from_artifact.

Related

Closes #7266
paritytech/substrate#13861

@mrcnski mrcnski self-assigned this May 22, 2023
@mrcnski mrcnski added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. T4-parachains_engineering This PR/Issue is related to Parachains performance, stability, maintenance. labels May 22, 2023
Copy link
Contributor

@s0me0ne-unkn0wn s0me0ne-unkn0wn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks very good! The only nit is naming: although the transition from artifact_path to artifact_bytes seems natural, we're usually using another naming for binary blobs, such as code, blob, code_blob, and similar. I'd use something like native_blob or I don't know. artifact_bytes says nothing about what's inside, I mean, that's a native executable code, not just bytes. So consider improving naming here, but that's not a blocker at all.

@mrcnski
Copy link
Contributor Author

mrcnski commented May 23, 2023

@s0me0ne-unkn0wn artifact_blob?

@s0me0ne-unkn0wn
Copy link
Contributor

artifact_blob?

Maybe 🤔 Or compiled_blob. Or prepared_blob. Or executable_blob. Sometimes it's easier to roll the dice 😁

@mrcnski
Copy link
Contributor Author

mrcnski commented May 23, 2023

bot merge

@paritytech-processbot paritytech-processbot bot merged commit d715d3a into master May 23, 2023
@paritytech-processbot paritytech-processbot bot deleted the mrcnski/instantiate-runtime-from-bytes branch May 23, 2023 18:56
ordian added a commit that referenced this pull request May 24, 2023
* master:
  PVF: instantiate runtime from bytes (#7270)
ordian added a commit that referenced this pull request May 25, 2023
…slashing-client

* ao-past-session-slashing-runtime:
  XCM: Tools for uniquely referencing messages (#7234)
  Companion: Substrate#13869 (#7119)
  Companion for Substrate#14214 (#7283)
  Fix flaky test and error reporting (#7282)
  impl guide: Update Collator Generation (#7250)
  Add staking-miner bin (#7273)
  metrics: tests: Fix flaky runtime_can_publish_metrics (#7279)
  [companion] Fix request-response protocols backpressure mechanism (#7276)
  PVF: instantiate runtime from bytes (#7270)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. T4-parachains_engineering This PR/Issue is related to Parachains performance, stability, maintenance.
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

PVF: instantiate runtime from bytes
3 participants